refactor rl config: 1) remove unnecessary params; 2) model_config consistency #926
Conversation
Hanjun-Dai
requested review from
abheesht17,
hgao327,
jiangyangmu,
lc5211,
sizhit2,
tianshub and
wang2yn84
as code owners
|
Thank you for your PR! Can you add the correspondent test cases to tunix/cli/config_test.py? Thank you! |
|
Also, can you provide the command to generate the "current" model config? Do you overwrite the model of the base config? |
…config for consistency
Hanjun-Dai
force-pushed
the
clean_rl_config_cli
branch
from
4e2e19b to
4eaa49e
Compare
working on it! |
so the existing scripts should still work. If there's no direct modification to model_config (e.g., through cli), then model_config will take the value from the reference_model_config (reference_model_config is what's being used right now when setting up RL) |
|
closing this one as #960 is providing a better solution |
2 participants
There are actor_model_config, reference_model_config and rollout_model_config where there value might conflict with each other, if user don't overwrite them all explicitly. The benefits are:
Consistency
Current
where user overwrites the model to qwen-0.6b but the rest of the configs still show llama-8b, which is very confusing.
After this patch
Reusing checks in cli/config.py
A lot of checks / validations are based on model_config. Right now it seems for RL the config is set through reference_model_config. So to make it consistent, we can overwrite the model_config when no explicit configuration are set to model_config.
Checklist